-
Notifications
You must be signed in to change notification settings - Fork 242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert softmax from CTest to GTest #3479
Conversation
CompareResults(tensorGpuDataBackward, tensorCpuDataBackward); | ||
} | ||
|
||
std::vector<T> GetForwardCpu() const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we also have mloSoftmaxForwardRunHost and mloSoftmaxBackwardRunHost too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this one is parallel may be we can move this mloSoftmaxForwardRunHost and mloSoftmaxBackwardRunHost to avoid duplications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi, @bghimireamd
In C-test I did not see calls of these functions. Scope of my refactoring was to replace c-test with g-test.
If I misunderstand something, lets discuss.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or do you mean that we need to enhance test coverage for softmax? If yes I would prefer to do it in separate PR/ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah separate PR/ticket make more sense here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
CTest to GTest conversion for Softmax
soft_max.cpp file is detected as renamed (moved), so probably will not be easy to review.